Refetch shipping on coupon application - #1441

Regression: a persisted shipping selection is overwritten on load. On first render lastShippingMethodsKeyRef.current is null, so selectShippingMethod treats the rates as changed and picks the cheapest method. It then mutates the order if the saved line differs.

To reproduce, load a draft order with weight-based ($1) already selected and the default rates. main sends no ApplyCheckoutSessionShippingMethod; this branch applies free-shipping. This affects reloads, returning from redirect payment flows (MercadoPago/CCAvenue), and any remount of this form.

The free-order test was rewritten to expect this behavior, and the new "preserves an existing rate" test only uses the cheapest rate, so it can't catch it.

Suggested fix: on first load, seed the key from the current methods so an existing service code is kept, e.g. previousMethodsKey: lastShippingMethodsKeyRef.current ?? getShippingMethodsKey(shippingMethods) when existingMethod is set.

An explicit customer choice is downgraded whenever any rate in the list changes. The key includes cost, so any repricing counts as a change and falls back to availableMethods[0].

To reproduce, the customer picks Express ($20 vs Standard $5), then edits the postal code so the rates become $21 / $6. main keeps Express; this branch switches to Standard. Carrier rates reprice on almost every address edit, so this will be common. The same thing happens when a coupon makes a different method cheaper.

Is that intended? If not, I'd keep the current method whenever it's still available and the customer chose it. Only fall back to the cheapest when the current method is gone, was auto-defaulted, or free shipping newly appears. One way is to track user selection in handleValueChange.

I don't think this refetch can reflect the coupon. DraftOrderShippingRatesQuery takes only destination, and a wallet-entered coupon is never written to the draft order. It only feeds the read-only calculatedAdjustments query. So the rates come back the same as before.

Meanwhile this adds a round trip on every coupon change. Combined with the block at ~L847, it replaces the wallet's shippingMethods while the line items and total still use godaddyTotals.shipping.value / shippingMethod. If the wallet resets the selection to the first (cheapest) option, the displayed method and the charged amount diverge. I'd remove this block and the one at ~L847.

This can mutate selectedShippingRate outside a wallet event. shippingAddress is reset in handleClick and handleCancel, so this branch only runs while the sheet is open. When it does run, it changes the selected rate without resolving anything to the Express Checkout Element. handleConfirm would then submit a shippingTotal and method different from what the customer saw. Since Stripe ECE has no coupon entry, I'd drop the shipping refetch from this effect entirely.

Keying only on the discount codes removes the duplicate requests, but calculatedAdjustmentsRef depends on the subtotal and shipping too. If the cart or shipping changes while the codes stay the same, the cached adjustments are stale (e.g. a percentage discount computed on the old subtotal). Consider adding totals.subTotal / shippingTotal values to the key. The same applies to the Stripe effect.

Two questions:

1. Can the API return calculatedShippingRates: null for a legitimate "no rates" case, e.g. a store with no shipping profile? If so, this now shows the failure/retry UI and clears the applied shipping instead of "No shipping methods found."
2. Since this is an intentional throw, please set retry: false here explicitly. Hosts can pass their own QueryClient, and TanStack's default of 3 retries means ~7s of skeleton, plus useReconcileAfterDiscount awaits the refetch while the coupon button spins.

Pre-existing, but easy to fix now: this re-applies only order and shipping-line codes. The discount mutation replaces the whole list (DiscountStandalone sends the full set, and removal sends []), so an order with both an order-level code and a line-item code loses the line-item code on any shipping change. getDraftOrderDiscountCodes(order) would fix it. DiscountStandalone could use the same helper.

This hook isn't used anywhere. It's still edited here, and it calls useDiscountApply, which now runs useReconcileAfterDiscount. That could re-apply a shipping method right after it's removed. I'd delete it, or switch it to useApplyDiscountCore.

The billing bullets (this line through L10) already shipped in #1438 (e366269), and they're in the @godaddy/react CHANGELOG. Please drop them so the release notes don't repeat them.

Nit: the rest of deDe uses formal address ("Sie"). Suggest 'Versandarten konnten nicht geladen werden. Bitte versuchen Sie es erneut.'

Regression: infinite ApplyCheckoutSessionShippingMethod loop when an automatic selection is switched after an address edit.

Repro: fresh checkout with no saved shipping line (standard $5 is auto-selected), the customer edits the postal code, and the new rates are express $20 / standard $25. On main this sends exactly one mutation (keeps standard, repriced). On this branch the mutations alternate express → standard → express … indefinitely.

Mechanism (from instrumenting the effect):

1. This effect runs form.setValue('shippingMethod', 'express', { shouldDirty: false }) and mutate(express).
2. In the same commit, the parent CheckoutForm hydration effect runs with a stale isCheckoutBusy === false (the mutation only just started) and a draft-order snapshot whose line is still standard, so form.reset(...) writes shippingMethod = 'standard'. The value isn't dirty, so keepDirtyValues doesn't protect it.
3. The mutation succeeds: server/cache = express, form = standard. The rates haven't changed, so selectShippingMethod keeps the "current" method, and this line picks the form value → mutate(standard) → repeat.

main is immune because it never changes the method automatically, so the form and server never diverge non-dirtily. Explicit clicks are dirty and protected, which is why the "customer-chosen Express + reprice" test passes. The same loop triggers via the "free shipping newly appears" rule when the current method came from the server on load (also non-dirty).

Fix I validated locally (full checkout suite still passes, repro converges): an automatic selection should never outrank the order's shipping line.

const existingMethod = previousAutoSelected
  ? currentServiceCode || currentFormMethod
  : currentFormMethod || currentServiceCode;

Please also add a regression test; none of the current tests exercise an address-triggered switch. This is the one I used (it times out on the current branch because isMutating never settles, and passes with the fix):

it('moves an automatic selection to the new cheapest rate when the address reprices it', async () => {
  const rates = (standard: number, express: number) =>
    buildShippingRates([
      { serviceCode: 'standard', carrierCode: 'carrier', displayName: 'Standard', cost: { value: standard, currencyCode: 'USD' } },
      { serviceCode: 'express', carrierCode: 'carrier', displayName: 'Express', cost: { value: express, currencyCode: 'USD' } },
    ]);
  const { user, queryClient } = renderCheckout({
    apiOverrides: { shippingMethods: rates(500, 2000) },
    draftOrderOverrides: { shippingLines: [] },
  });
  await waitForCheckoutReady();
  await waitFor(() => expect(queryClient.isMutating()).toBe(0));
  expect(screen.getByRole('radio', { name: /standard/i })).toBeChecked();
  clearOperations();

  setShippingMethods(rates(2500, 2000)); // express is now the cheapest
  const postal = document.querySelector('input[name="shippingPostalCode"]') as HTMLInputElement;
  await user.clear(postal);
  await user.type(postal, '94016');
  await advanceCheckoutDebounce();
  await waitForOperation('UpdateCheckoutSessionDraftOrder');
  await waitForOperation('DraftOrderShippingRates', 1, 6000);
  await waitFor(() => {
    expect(queryClient.isMutating()).toBe(0);
    expect(queryClient.isFetching()).toBe(0);
  });
  await flushPromises();

  const applied = getOperations('ApplyCheckoutSessionShippingMethod');
  expect(applied.at(-1)?.input).toEqual([expect.objectContaining({ requestedService: 'express' })]);
  expect(screen.getByRole('radio', { name: /express/i })).toBeChecked();
});

Still think this should go. The PR description cites checkout-api#207, but that change computes adjustedOrderTotal from discounts persisted on the draft order. A wallet-entered coupon is never persisted — it only feeds calculatedAdjustments, and close_wallet discards it — so this refetch returns the same rates as the shipping_address_change fetch both before and after #207. It's a guaranteed no-op round trip on every coupon change.

The risk from round one also stands: updateWith({ shippingMethods }) (~L851) replaces the wallet's list while lineItems/total in the same update are computed from the old godaddyTotals.shipping. If Poynt resets the selection to the first entry, the displayed method and the charged amount diverge.

Please remove this block and the one at ~L851, or confirm Poynt preserves the selected method on updateWith({ shippingMethods }).

See the comment at ~L549. This is the half that can desynchronise the wallet's selected method from godaddyTotals.shipping.

Round-one question still open: can the API return calculatedShippingRates: null for a legitimate "no shipping profile configured" store? If so, this now shows the failure/retry UI and clears the applied shipping line instead of "No shipping methods found." A quick confirmation either way is enough.

Non-blocking design note. A transient rates failure during coupon apply now clears the order's shipping line and, on recovery, defaults to the cheapest method (the ['error', 'default'] case documents Express → Standard). On main, coupon apply had no dependency on the rates endpoint at all.

It's a deliberate choice and it's tested, so I won't block on it, but please add a short comment here explaining why clearing is preferred over keeping the line and blocking payment until retry. A cheap improvement: don't clear form.shippingMethod in this path, so recovery re-applies the customer's method instead of the cheapest.

Nit: this MercadoPago string change (du → Sie) is unrelated to the PR. Fine to keep, but please mention it in the changeset or split it out.

Nit (non-blocking): useApplyShippingMethod now passes codes in sorted order, whereas main sent order-level then shipping-level. Harmless if the discount API is order-insensitive; flagging in case it isn't.